Skip to content

[docs]: Update test website code #16084

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jul 24, 2025
Merged

[docs]: Update test website code #16084

merged 1 commit into from
Jul 24, 2025

Conversation

@navin772
Copy link
Member

@navin772 navin772 commented Jul 24, 2025 •
edited by qodo-merge-pro bot
Loading

User description

🔗 Related Issues

💥 What does this PR do?

Updates the test website, there were some typos in the site like Long instead of Log in - https://www.selenium.dev/selenium/web/logEntryAdded.html and others which were fixed in previous commits but were not updated in the gh-pages branch.

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

  • Cleanup (formatting, renaming)

PR Type

Documentation

Description

  • Fix typo in log entry test pages

  • Add new FedCM and service worker test pages

  • Improve relative locators HTML structure and styling

  • Update form page select options visibility

Diagram Walkthrough

flowchart LR
  A["Typo Fixes"] --> B["Test Pages"]
  C["New Pages"] --> B
  D["HTML Structure"] --> B
  B --> E["Updated Test Website"]
Loading

File Walkthrough
Relevant files

@qodo-merge-pro
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ❌

5678 - Not compliant

Non-compliant requirements:

  • Fix ChromeDriver connection failure error that occurs after first instance
  • Address "ConnectFailure (Connection refused)" error for subsequent ChromeDriver instances
  • Ensure proper ChromeDriver instantiation on Ubuntu 16.04.4 with Chrome 65.0.3325.181

1234 - Not compliant

Non-compliant requirements:

  • Fix JavaScript execution in link href on click() method for Selenium 2.48
  • Ensure JavaScript alerts are triggered properly in Firefox 42.0
  • Restore functionality that worked in version 2.47.1 but broke in 2.48.0/2.48.2
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Hardcoded URL

The configURL construction uses template literals with location.host which may not work correctly in all test environments and could cause issues with different host configurations.

 
let configURL = `http://${location.host}/common/fedcm/config.json`;
console.log(configURL)
Path Change

The JavaScript package path has been changed from node/selenium-webdriver to selenium-webdriver which may break existing build dependencies or references.

 
"//javascript/selenium-webdriver:__pkg__",
"//py:__pkg__",

@qodo-merge-pro
Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Add error handling for registration

The service worker registration lacks error handling which could cause silent
failures. Add a catch block to handle registration errors properly.

web/service_worker.html [7-9]

 navigator.serviceWorker.register("/service-worker.js", { scope: "/" })
   .then(() => navigator.serviceWorker.ready)
-  .then(() => console.debug("[Companion]", "Service worker registered!"));
+  .then(() => console.debug("[Companion]", "Service worker registered!"))
+  .catch(error => console.error("[Companion]", "Service worker registration failed:", error));
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly adds a .catch block to handle potential errors during service worker registration, which is crucial for debugging and robust error handling.

Medium
Security
Use protocol-relative URL construction

Using http:// protocol in template literals can cause security issues and mixed
content warnings on HTTPS sites. Consider using protocol-relative URLs or
checking the current protocol.

web/fedcm/fedcm_async_js.html [12]

-let configURL = `http://${location.host}/common/fedcm/config.json`;
+let configURL = `${location.protocol}//${location.host}/common/fedcm/config.json`;
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies that hardcoding http:// can cause mixed-content issues and proposes a robust fix using location.protocol.

Low
  • More

@navin772 navin772 requested a review from diemol July 24, 2025 07:02
@navin772
Copy link
Member Author

web/service-worker.js and html were added in #15416.

@diemol diemol merged commit 6af8c2c into SeleniumHQ:gh-pages Jul 24, 2025
1 check passed
@navin772 navin772 deleted the gh-pages branch July 24, 2025 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@diemol diemol Awaiting requested review from diemol

Assignees

No one assigned

Labels

Review effort 2/5

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@navin772 @diemol